Make viscosity inplace and avoid duplicate velocity loads#1125
Merged
svchb merged 17 commits intotrixi-framework:mainfrom Apr 13, 2026
Merged
Make viscosity inplace and avoid duplicate velocity loads#1125svchb merged 17 commits intotrixi-framework:mainfrom
svchb merged 17 commits intotrixi-framework:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1125 +/- ##
===========================================
+ Coverage 67.03% 88.85% +21.82%
===========================================
Files 128 128
Lines 9808 9864 +56
===========================================
+ Hits 6575 8765 +2190
+ Misses 3233 1099 -2134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced Apr 1, 2026
…les.jl into inplace-viscosity
There was a problem hiding this comment.
Pull request overview
Refactors viscosity evaluation to support in-place accumulation and reduce redundant velocity loads in SPH interaction kernels (targeting GPU performance improvements as part of #1131).
Changes:
- Replaces return-by-value viscosity contributions with in-place accumulation via
dv_viscosity!(dv_particle_ref, ...). - Preloads
v_a/v_band threads them through viscosity/viscous_velocityAPIs to avoid duplicatecurrent_velocityloads. - Updates tests and removes an unused test import.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/schemes/fluid/viscosity.jl |
Introduces dv_viscosity! in-place API; updates all viscosity models to accumulate into a Ref; adds viscous_velocity fast-path using preloaded velocities. |
src/schemes/fluid/weakly_compressible_sph/rhs.jl |
Preloads velocities and switches viscosity term computation to dv_viscosity!. |
src/schemes/fluid/implicit_incompressible_sph/rhs.jl |
Preloads velocities and switches viscosity term computation to dv_viscosity!. |
src/schemes/fluid/implicit_incompressible_sph/system.jl |
Updates predicted-velocity step to use dv_viscosity! (in-place) and dereference via dv_viscosity_[]. |
src/schemes/fluid/entropically_damped_sph/rhs.jl |
Preloads velocities and switches viscosity term computation to dv_viscosity!. |
src/schemes/boundary/open_boundary/rhs.jl |
Switches boundary viscosity handling to dv_viscosity! and uses preloaded velocities. |
src/schemes/boundary/open_boundary/system.jl |
Avoids duplicate velocity loads by passing preloaded velocity into viscous_velocity. |
src/general/interpolation.jl |
Avoids duplicate velocity loads by passing preloaded velocity into viscous_velocity. |
src/schemes/boundary/wall_boundary/system.jl |
Updates viscous_velocity signatures to accept preloaded v_particle and become no-op when regular velocity is used. |
src/schemes/structure/rigid_body/system.jl |
Updates viscous_velocity signature to accept preloaded velocity. |
src/schemes/structure/total_lagrangian_sph/system.jl |
Updates viscous_velocity signature to accept preloaded velocity. |
src/schemes/structure/structure.jl |
Preloads velocities and switches structure–fluid viscosity term computation to dv_viscosity!. |
test/schemes/fluid/viscosity.jl |
Updates viscosity tests to use in-place API and dereference Ref results. |
test/general/density_calculator.jl |
Removes unused OrdinaryDiffEq import. |
Comments suppressed due to low confidence (3)
src/schemes/fluid/viscosity.jl:253
smoothing_length_neighboris taken fromparticle_system, butneighboris part ofneighbor_system. For mixed-system interactions this will computesmoothing_length_averageincorrectly. Usesmoothing_length(neighbor_system, neighbor)for the neighbor smoothing length.
smoothing_length_particle = smoothing_length(particle_system, particle)
smoothing_length_neighbor = smoothing_length(particle_system, neighbor)
src/schemes/fluid/viscosity.jl:344
smoothing_length_neighbor = smoothing_length(particle_system, neighbor)uses the particle system for the neighbor particle. Whenneighbor_systemdiffers (e.g., boundaries/structures), this will use the wrong smoothing length in the SGS viscosity. Usesmoothing_length(neighbor_system, neighbor)here.
epsilon = viscosity.epsilon
smoothing_length_particle = smoothing_length(particle_system, particle)
smoothing_length_neighbor = smoothing_length(particle_system, neighbor)
src/schemes/fluid/viscosity.jl:463
smoothing_length_neighboris computed fromparticle_system, butneighborbelongs toneighbor_system. This can produce incorrectsmoothing_length_averagefor mixed-system viscosity interactions. Usesmoothing_length(neighbor_system, neighbor)instead.
epsilon = viscosity.epsilon
smoothing_length_particle = smoothing_length(particle_system, particle)
smoothing_length_neighbor = smoothing_length(particle_system, neighbor)
smoothing_length_average = (smoothing_length_particle + smoothing_length_neighbor) / 2
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LasNikas
previously approved these changes
Apr 9, 2026
svchb
previously approved these changes
Apr 13, 2026
Collaborator
|
/run-gpu-tests |
svchb
approved these changes
Apr 13, 2026
Member
Author
|
/run-gpu-tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of #1131 and
dv_viscosityover all neighbors before writing it todv(with Improve GPU performance of fluid-* interact #1116),viscous_velocityno-ops when the regular velocity is used, which we already have to load for the continuity equation,